Skip to content

fix(session): don't clobber user's model swap on model-less prompts - #42894

Open
Qiiks wants to merge 1 commit into
anomalyco:devfrom
Qiiks:pr/model-swap-survives-modelless-prompts
Open

fix(session): don't clobber user's model swap on model-less prompts#42894
Qiiks wants to merge 1 commit into
anomalyco:devfrom
Qiiks:pr/model-swap-survives-modelless-prompts

Conversation

@Qiiks

@Qiiks Qiiks commented Aug 16, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #42893

Type of change

  • Bug fix

What does this PR do?

A model-less prompt (no input.model — e.g. plugin completion-reminders via promptAsync) resolves from the agent fallback (ag.model) for the turn. The existing design (PR #26765) intentionally uses the agent's model when no explicit model is given — this is correct for the turn's model and is preserved.

However, setAgentModel then overwrites the session row to the resolved model, permanently destroying a model the user explicitly stored via a prior swap. The TUI is unaffected (always sends draft.model explicitly), but opencode serve / mobile clients lose the swap on the next plugin reminder.

The fix: guard the setAgentModel write in createUserMessage — skip it when:

  1. The prompt carried no explicit model (input.model == null), AND
  2. The session already has a stored model that differs from the agent's resolved model (storedModelDiffers)

Fresh sessions (no stored model) still persist the agent's model on the first model-less prompt (hasNoStoredModel guard), and explicit-model prompts always persist normally. The model-resolution precedence is unchanged — model-less prompts still use the agent's model for the turn.

How did you verify your code works?

  • New regression test: "model-less prompt does not clobber user's stored model swap" — creates a session, swaps to kimi-k2.5-free, sends a model-less prompt with an agent carrying test/test-model, asserts the turn uses test-model (design preserved) but the session row stays kimi-k2.5-free (the fix). Verified RED→GREEN.
  • Fresh-session test: "model-less first prompt on fresh session persists agent model to row" — confirms the hasNoStoredModel guard fires setAgentModel on a fresh session so the row is initialized.
  • Existing test preserved: "applies agent variant only when using agent model" (the test that pins the agent-model design from Persist session model switches outside event flag #26765) still passes — model-less prompts still use the agent's model for the turn.
  • Full suite: 46 pass / 14 skip / 0 fail across test/session/prompt.test.ts.
  • Typecheck: 30/30 packages clean (turbo typecheck).

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@Qiiks
fix(session): don't clobber user's model swap on model-less prompts
bf3eaef 
A model-less prompt (no input.model — e.g. plugin completion-reminders
via promptAsync) resolves from the agent fallback (ag.model) for the turn.
The existing design (PR anomalyco#26765) intentionally uses the agent's model when
no explicit model is given.

However, setAgentModel then overwrites the session row to the resolved
model, permanently destroying a model the user explicitly stored via a
prior swap. The TUI is unaffected (always sends draft.model explicitly),
but opencode serve / mobile clients lose the swap on the next plugin
reminder.

Guard the setAgentModel write: skip it when the prompt carried no
explicit model AND the session already has a stored model that differs
from the agent's resolved model. Fresh sessions (no stored model) and
explicit-model prompts still persist normally.
Copilot AI lite review requested due to automatic review settings August 16, 2026 13:26
Copilot AI reviewed Aug 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@HermanGrondahl

HermanGrondahl commented Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown

This seems to fix the original model-clobber case, but I found a related state-update edge case: the guard can also suppress an explicit agent change because setAgentModel() persists the agent and model together.

Consider this sequence:

  • stored session: agent = plan, model = A
  • A was explicitly selected by the user
  • next prompt explicitly selects agent = build
  • the prompt has no explicit input.model
  • build resolves fallback model B

The PR correctly prevents fallback model B from overwriting the explicitly stored model A.

However, because wouldClobberStoredModel skips the entire setAgentModel() call, the explicit agent change is skipped as well. The persisted session remains:

plan + A

rather than:

build + A

In this case the two fields need different treatment: preserve the stored model, but still persist the explicitly selected agent.

I think a regression covering this transition would be useful:

  • before: plan + A
  • input: explicit build, no explicit model
  • build fallback: B
  • after: build + A

That would preserve the model-swap fix without leaving the persisted agent stale.

@Enough1122

Enough1122 commented Aug 21, 2026

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  • packages/opencode/src/session/prompt.ts:679 — logic checks out for the four model/agent combinations (explicit model always persists; model-less + stored-model-differs skips; fresh session initializes). Good minimal fix that preserves the Persist session model switches outside event flag #26765 turn-resolution design.
  • prompt.ts:682 — edge case worth a decision: when input.model == null and the agent also changed (current.agent !== info.agent), wouldClobberStoredModel blocks the whole setAgentModel write, so the agent switch isn't persisted either. Since setAgentModel writes both fields together, you can't skip just the model today — but silently dropping an agent change may surprise callers. Consider persisting agent while keeping the stored model, or documenting the tradeoff.
  • prompt.ts:24 — comment clearly explains intent; nice.
  • Tests: both new instance tests assert the important invariant pair (turn uses agent model, row keeps user swap) plus fresh-session initialization. Thorough.
  • Nit: test/session/prompt.test.ts:2443 — double blank line introduced at top of the new block; prettier will likely flag. No blocking issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Model swap gets clobbered by model-less prompts (plugin reminders)

4 participants

@Qiiks @HermanGrondahl @Enough1122